-
Notifications
You must be signed in to change notification settings - Fork 66
omdb db sql
#9554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
omdb db sql
#9554
Conversation
davepacheco
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm very excited about this.
CockroachDB does have a default_transaction_read_only option. And it looks like you can pass --read-only to cockroach sql (and so presumably cockroach-sql):
--read-only
Set the session variable default_transaction_read_only to on.
But even if we do this for you, the user can just change it once you're in the SQL shell. I think what I'd suggest is to do both: require -w/--destructive in omdb and also pass --read-only when invoking cockroach-sql. I could be talked into not requiring the -w on omdb, on the argument that "if you set read-only off in the interactive shell, that's on you".
| source.packages = [ | ||
| "omicron-nexus.tar.gz", | ||
| "omicron-omdb.tar.gz", | ||
| "cockroachdb-sql.tar.gz", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this in the Nexus zone?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
omdb is in the Nexus zone too. If we don't add it then omdb db sql will fail to exec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we expect to ever run it from there, but fair enough. It seems harmless, anyway.
| source.packages = [ | ||
| "omicron-nexus.tar.gz", | ||
| "omicron-omdb.tar.gz", | ||
| "cockroachdb-sql.tar.gz", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we expect to ever run it from there, but fair enough. It seems harmless, anyway.
Co-authored-by: David Pacheco <[email protected]>
Closes #9021.
Changes to oxidecomputer/cockroachdb (no source changes):
oxidecomputer/cockroach@80eed95
oxidecomputer/cockroach@367bca4